-
Notifications
You must be signed in to change notification settings - Fork 191
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add --in-place flag to verdi export migrate #4220
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #4220 +/- ##
===========================================
- Coverage 79.34% 79.32% -0.01%
===========================================
Files 468 468
Lines 34737 34752 +15
===========================================
+ Hits 27558 27565 +7
- Misses 7179 7187 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ltalirz some small changes
0411220
to
f2bb103
Compare
f96ca55
to
20c44bb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine to me.
I have some suggested updates to parts of the tests, nothing serious.
What's more serious is that you are effectively implementing/changing two things in this PR, one is adding the --in-place
flag option, but then you're also changing expected behaviour of the migration functionality.
The latter might be fine, but where has this change been discussed?
425527f
to
268f84e
Compare
268f84e
to
a9e38e2
Compare
@sphuber This should be ready for review |
a9e38e2
to
7a7671d
Compare
When export migration is required, you can often get rid of the outdated export file. The `--in-place` flag saves users from specifying a temporary filename, removing the old file and renaming the temporary one. Using a temporary directory avoids the use of protected tempfile functions and guarantees that there are no filename clashes. This commit also changes the exit code for migrating an export file that already is up to date from an error to success (0). Co-authored-by: Sebastiaan Huber <[email protected]> Co-authored-by: Casper Welzel Andersen <[email protected]>
7a7671d
to
00c89b1
Compare
Fixes #4233
Fixes #4322
When export migration is required, you often want to get rid of the outdated
export file.
The
--in-place
flag saves users from specifying a temporary filename,removing the old file and renaming the temporary one.
This commit also changes the exit code for migrating an export file that
already is up to date from an error to success (0).